Fix missed CompressPointer reset in EndSort when without-zlib #593
Conversation
Reset of *AR.CompressPointer was the wrong side of an #ifdef WITHZLIB. Also fix the --without-zlib build, broken since the split sort allocation patch: 7d0e71d
There was a problem hiding this comment.
I have confirmed that this PR fixes #95 (comment) (or
#95 (comment)) without zlib. (The source code remains unchanged with zlib.)
Is the suffix b in the test case name Issue95b intended to suggest a partial fix?
|
Yes exactly, this issue is not really anything too do with the original gzip issue of #95 (fixed in 26793e4), we just found it there while testing. A test case was never added for the original gzip but, I will add one as part of this PR. I just need to check how long the test case I found takes to run under valgrind, maybe it needs further trimming down to be suitable. Regarding the further reports of the gzip bug, I never managed to reproduce any of them. Looking back at the discussion, no one actually confirmed that they were using a post - 26793e4 binary. It is possible, that #95 can now be closed... |
jodavies
force-pushed
the
fix-issue-95b
branch
from
114c4af to
9c29d6e
Compare
jodavies
force-pushed
the
fix-issue-95b
branch
from
9c29d6e to
1c7c1af
Compare
|
Thanks. Fixed those. |
This is not true actually, @alonli1 's log files are FORM 5 beta. Also of note, is that those two subsequent reports are zerror -3 and not -2 like the original report. This issue should remain open I suppose... |
jodavies
force-pushed
the
fix-issue-95b
branch
from
1c7c1af to
76d08ac
Compare
This was fixed a long time ago, but no test case was added. Add one now, just in case. This fails pre 26793e4 for form and tform -w2, but not tform -w4. Skip it for valgrind tform, since it takes a long time to run.
jodavies
force-pushed
the
fix-issue-95b
branch
from
76d08ac to
46b1fbf
Compare
|
I feel the discussions in #95 have become rather lengthy. Maybe we could extract and summarize the remaining problems and create a new issue for them, then this PR could close it. |
|
Yes, that works for me. I think it is also not impossible that |
3 participants
Reset of *AR.CompressPointer was the wrong side of an #ifdef WITHZLIB.
Also fix the --without-zlib build, broken since the split sort allocation patch: 7d0e71d
Partially closes #95 ?